Skip to content

Fix reinit synchronization#13

Merged
febyeji merged 15 commits intofebyeji:cbf-integrationfrom
randomlogin:cbf-fix-reinit-sync
Apr 24, 2026
Merged

Fix reinit synchronization#13
febyeji merged 15 commits intofebyeji:cbf-integrationfrom
randomlogin:cbf-fix-reinit-sync

Conversation

@randomlogin
Copy link
Copy Markdown

Fixes a hang in start/stop/reinit after a checkpoint-based resume where wait_for_cbf_sync timed out because neither
sync timestamp advanced.

  • sync_lightning_wallet: when no scripts are registered, still call update_node_metrics_timestamp so callers waiting
    on a strict advance of latest_lightning_wallet_sync_timestamp make progress. The empty-scripts and normal paths now share a single timestamp-update site at the end of the closure.
  • fee_rate_cache_from_cbf: treat FetchBlockError::UnknownHash as a skip-this-cycle condition instead of an error. After a checkpoint resume, requester.chain_tip() can return the checkpoint hash which has no BlockNode in kyoto's tree yet, previously surfacing as FeerateEstimationUpdateFailed from node.start().

Co-Authored-By: Claude Opus 4.7 noreply@anthropic.com

@febyeji
Copy link
Copy Markdown
Owner

febyeji commented Apr 21, 2026

Thanks for the work! Could you split this into two commits? Fix 1 (timestamp progression) and Fix 2 (UnknownHash) are logically independent, so it would be helpful to bisect them.

@randomlogin randomlogin force-pushed the cbf-fix-reinit-sync branch from 0785f6a to 063a7aa Compare April 21, 2026 09:43
febyeji and others added 15 commits April 22, 2026 17:42
Fixes the problem when the funding tx is not registered for CBF node, for
example during splicing.

Added macros to skip incompatible with CBF backend tests (which require
mempool).
Decrease timeouts to not falsely conclude some tests as passing
Previously we tracked synced height and also had dirty flag for rescan
which was made from the genesis block. This was removed in favor of
block heights from channel manager and onchain wallet.

Safety reorg interval of 7 blocks (look-back) is added.

Force tests use CBF config with 1 required peers, as otherwise it would
die if one of two required peers went offline (which made impossible
testing of scenarios with 2 nodes on which goes offline).
  add auto-restart with exponential backoff (lightningdevkit#10)

* refactor(cbf): extract build_cbf_node helper and add wallet reference
* feat(cbf): auto-restart bip157 node with exponential backoff
* feat(cbf): add liveness check before returning requester
* fix(cbf): clean up scan state on filter scan failure
* fix(cbf): add per-block-fetch timeout to wallet sync
The old wait_for_cbf_sync only checked that the onchain sync timestamp
advanced, which could false-pass when the timestamp updated but wallet
state was still stale. The new version verifies both onchain and
lightning wallet syncs completed, and accepts a check closure so callers
can assert concrete wallet state (e.g. balance, payment existence).

Also simplifies CbfSyncConfig in test setup to use Default (which
already sets required_peers: 1 and reasonable timeouts).
`push` only appends above the tip, so when `recent_history` contained
blocks at or below the wallet's current checkpoint height after a reorg,
the stale hashes on the wallet checkpoint were never replaced. Switch to
`CheckPoint::insert`, which detects conflicting hashes and purges stale
blocks, matching bdk-kyoto's `UpdateBuilder::apply_chain_event`.

Also clear `latest_tip` on `BlockHeaderChanges::Reorganized` so cached
tip state does not point at an abandoned chain.

Update the `checkpoint_building_handles_reorg` unit test (added in
c1844b3) to exercise the fixed behaviour: a reorg where the new tip
is at the same height as the wallet's checkpoint must still result
in the reorged hashes winning.

Disclosure: drafted with assistance from Claude Code.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…istered

`sync_lightning_wallet` returned early without updating
`latest_lightning_wallet_sync_timestamp` when no scripts were registered,
causing `wait_for_cbf_sync` to hang on start/stop/reinit cycles. Fold the
empty-scripts branch into the main path so both share a single
`update_node_metrics_timestamp` site at the end of the closure.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…_cbf

After a checkpoint-based resume, `requester.chain_tip()` can return the
checkpoint hash which has no `BlockNode` in kyoto's tree yet, causing
`get_block` to fail with `FetchBlockError::UnknownHash` and surfacing as
`FeerateEstimationUpdateFailed` from `node.start()`. Skip the cycle and
retry later instead of erroring.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…s after upstream API change

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@randomlogin randomlogin force-pushed the cbf-fix-reinit-sync branch from 063a7aa to 6a5b69e Compare April 22, 2026 15:47
@febyeji febyeji force-pushed the add-cbf-chain-source branch 2 times, most recently from afac435 to 9a3152e Compare April 24, 2026 00:30
@febyeji febyeji changed the base branch from add-cbf-chain-source to cbf-integration April 24, 2026 01:13
@febyeji febyeji merged commit 7fdd793 into febyeji:cbf-integration Apr 24, 2026
@randomlogin randomlogin deleted the cbf-fix-reinit-sync branch April 24, 2026 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants